Skip to content

Conversation

@joshblack
Copy link
Member

@joshblack joshblack commented Nov 17, 2025

Unfortunately our labeling workflow won't work if the PR is one from a fork 😞 Trying out one related to issue_comment so that we could have CI pass on PRs from forks.

@changeset-bot
Copy link

changeset-bot bot commented Nov 17, 2025

⚠️ No Changeset found

Latest commit: 1d2f65c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the staff Author is a staff member label Nov 17, 2025
@joshblack joshblack added the skip changeset This change does not need a changelog label Nov 17, 2025
Copilot finished reviewing on behalf of joshblack November 17, 2025 17:50
@joshblack joshblack added this pull request to the merge queue Nov 17, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for triggering status check overrides via issue comments, specifically to enable CI bypass for pull requests from forks where the label-based workflow doesn't work due to permission restrictions.

Key Changes:

  • Adds issue_comment event trigger to the workflow
  • Implements a new command-based flow using .skip-integration-checks comment
  • Updates permissions to include issues: write for comment interactions

Comment on lines +88 to +90
- name: Override status checks for issue comment
if: ${{ github.event_name == 'issue_comment' && steps.command.outputs.continue == 'true' }}
run: |
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue_comment event from forks is potentially unsafe because the workflow runs in the context of the base repository with elevated permissions (issues: write, pull-requests: write, statuses: write), but can be triggered by external contributors commenting on a PR.

While the github/command action provides some protection by requiring a specific command and permissions check, you should ensure that:

  1. The action properly validates that the commenter has the required permissions in the repository
  2. The permissions: write parameter correctly restricts to users with write access

Consider adding an explicit permission check or documenting the security model, especially since this is designed to work with PRs from forks where the PR author may not have write access.

Suggested change
- name: Override status checks for issue comment
if: ${{ github.event_name == 'issue_comment' && steps.command.outputs.continue == 'true' }}
run: |
- name: Check comment author permissions
if: ${{ github.event_name == 'issue_comment' && steps.command.outputs.continue == 'true' }}
id: author-permissions
run: |
AUTHOR="${{ github.event.comment.user.login }}"
PERMISSION=$(gh api "/repos/${{ github.repository }}/collaborators/$AUTHOR/permission" --jq '.permission')
echo "Author permission: $PERMISSION"
if [[ "$PERMISSION" == "admin" || "$PERMISSION" == "write" || "$PERMISSION" == "maintain" ]]; then
echo "has_write_access=true" >> $GITHUB_OUTPUT
else
echo "has_write_access=false" >> $GITHUB_OUTPUT
fi
env:
GH_TOKEN: ${{ steps.generate_primer_token.outputs.token }}
- name: Override status checks for issue comment
if: ${{ github.event_name == 'issue_comment' && steps.command.outputs.continue == 'true' && steps.author-permissions.outputs.has_write_access == 'true' }}
run: |

Copilot uses AI. Check for mistakes.
Comment on lines 88 to 103
- name: Override status checks for issue comment
if: ${{ github.event_name == 'issue_comment' && steps.command.outputs.continue == 'true' }}
run: |
SHA=$(gh pr view $NUMBER --json headRefOid --jq '.headRefOid')
gh api -X POST "/repos/primer/react/statuses/$SHA" \
-f state='success' \
-f context='github-ui / ci' \
-f description='Manual override' \
-f target_url="$COMMENT_URL"
gh api -X POST "/repos/primer/react/statuses/$SHA" \
-f state='success' \
-f context='github-ui / projects' \
-f description='Manual override' \
-f target_url="$COMMENT_URL"
Copy link

Copilot AI Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The step "Override status checks for issue comment" lacks error handling when the gh pr view command fails. If NUMBER refers to an issue instead of a pull request (which is possible with the current issue_comment trigger), this command will fail.

Consider adding error handling or verifying the command succeeded before proceeding with the API calls:

run: |
  if ! SHA=$(gh pr view $NUMBER --json headRefOid --jq '.headRefOid' 2>/dev/null); then
    echo "Failed to get PR information for issue #$NUMBER"
    exit 1
  fi
  # ... rest of the script

Copilot uses AI. Check for mistakes.
@joshblack joshblack removed this pull request from the merge queue due to a manual request Nov 17, 2025
joshblack and others added 2 commits November 17, 2025 11:54
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@joshblack joshblack added the integration-tests: skipped manually Changes in this PR do not require an integration test label Nov 17, 2025
@joshblack joshblack enabled auto-merge November 17, 2025 17:57
@github-actions github-actions bot requested a deployment to storybook-preview-7203 November 17, 2025 17:59 Abandoned
@joshblack joshblack added this pull request to the merge queue Nov 17, 2025
Merged via the queue into main with commit bcdb340 Nov 17, 2025
55 checks passed
@joshblack joshblack deleted the ci/add-issue-comment-to-status-checks branch November 17, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: skipped manually Changes in this PR do not require an integration test skip changeset This change does not need a changelog staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants